Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add a remote user profile cache #2429

Merged
merged 4 commits into from
Aug 25, 2017

Conversation

erikjohnston
Copy link
Member

We want to be able to return profiles of users who are in the group when requested, which ideally needs some sort of profile cache.

This is done by adding the user to a cache table and periodically going and checking if there are updates. To add a remote user to the table:

  1. Ensure that is_subscribed_remote_profile_for_user(user_id) returns true for the user
  2. Call add_remote_profile_cache with an up to date copy of their profile

is_subscribed_remote_profile_for_user ensures that we only bother fetching profiles for users we really do actually care about.

PRs to follow:

  • Using the remote cache to add profiles to /users API
  • Add federation pokes to inform servers that we think care that a users profile has changed

@erikjohnston erikjohnston changed the base branch from develop to erikj/groups_merged August 25, 2017 10:29
@erikjohnston
Copy link
Member Author

Sorry @NegativeMjark for splitting out the ProfileHandler in this PR, but it fixes the tests....

@@ -182,3 +220,44 @@ def _update_join_states(self, requester):
"Failed to update join event for room %s - %s",
room_id, str(e.message)
)

def _update_remote_profile_cache(self):
"""Called periodically to check profiles of remote users we havent'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

havent'

is_subcscribed = yield self.store.is_subscribed_remote_profile_for_user(
user_id,
)
if not is_subcscribed:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subcscribed

self.ratelimiter = hs.get_ratelimiter()

# AWFUL hack to get at BaseHandler.ratelimit
self.base_handler = BaseHandler(hs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I want to know why this is needed, but future generations deserve to know why we did what we did.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I actually I can still just inherit from BaseHandler. At this point I think I was close to purging all the unit tests with fire so may not have been at my best.....

Copy link
Contributor

@NegativeMjark NegativeMjark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM apart from spelling, grammar and justification.

@@ -55,3 +57,99 @@ def set_profile_avatar_url(self, user_localpart, new_avatar_url):
updatevalues={"avatar_url": new_avatar_url},
desc="set_profile_avatar_url",
)

def get_from_remote_profile_cache(self, user_id):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually called anywhere? Or is the cache write-only at the moment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is TBC...

@erikjohnston
Copy link
Member Author

@matrixbot restest this please

4 similar comments
@erikjohnston
Copy link
Member Author

@matrixbot restest this please

@erikjohnston
Copy link
Member Author

@matrixbot restest this please

@erikjohnston
Copy link
Member Author

@matrixbot restest this please

@erikjohnston
Copy link
Member Author

@matrixbot restest this please

@erikjohnston
Copy link
Member Author

@matrixbot retest this please

4 similar comments
@erikjohnston
Copy link
Member Author

@matrixbot retest this please

@erikjohnston
Copy link
Member Author

@matrixbot retest this please

@erikjohnston
Copy link
Member Author

@matrixbot retest this please

@erikjohnston
Copy link
Member Author

@matrixbot retest this please

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants